Skip to content

Move projection maths onto the projection variants - #86

Merged
bjmorgan merged 6 commits into
mainfrom
feature/projection-variants
Aug 16, 2026
Merged

Move projection maths onto the projection variants#86
bjmorgan merged 6 commits into
mainfrom
feature/projection-variants

Conversation

@bjmorgan

Copy link
Copy Markdown
Owner

Follow-up to #85. That PR introduced the Orthographic | Perspective sum type
but left the projection maths as match/isinstance dispatch spread across
ViewState and three renderers. This moves that maths onto the variants behind
a Projection abstract base class, so adding a third variant is a one-class
change rather than an edit to every dispatch site.

Why

Projection was data only, so every question about a projection -- how a
camera point maps to the screen, a sphere's silhouette radius, the worst-case
magnification, the bond-cap eye distance, whether a point reaches the eye plane
-- was answered by a match outside the type. There were four such dispatch
sites plus assert_never guards. Those are all projection concepts, so they
belong on the projection.

Projection becomes an ABC (the existing LegendItem pattern) with five
abstract methods; Orthographic and Perspective implement them. The dispatch
sites collapse to method calls, the assert_never guards are replaced by
definition-time enforcement (an incomplete variant cannot be instantiated), and
the types move into a new hofmann/model/projection.py -- returning
view_state.py to being about ViewState (434 to 254 lines).

Behaviour

Rendered output is unchanged, bit-for-bit, verified with a vertex-level harness
against main after every commit. Two internal cleanups ride along, both
behaviour-neutral: project_camera now returns xy only (the scale it
returned was discarded by every caller), and the duplicated
_foreshortening_distance helper collapses to an eye_distance property.

Notes for review

  • The two assert isinstance(proj, Perspective) sites sit inside
    if proj.reaches_eye_plane(...), which only Perspective returns True
    from -- so they are unreachable for a parallel variant and narrow the type
    for the warning's f-string. They are not dispatch a third variant would edit.
  • Projection.__abstractmethods__ is pinned by a test, so a method silently
    losing @abstractmethod fails rather than letting an incomplete variant
    through.
  • One interface method does not yet generalise beyond the two current variants:
    eye_distance is a scalar z-axis distance, which a genuinely oblique
    projection could not express. Left as-is deliberately rather than reshaped for
    an unwritten variant; the follow-up oblique work will generalise it.

Projection is now the ABC base rather than a union alias, so api.rst
uses autoclass.

The abstract-enforcement test previously used an empty subclass, which
stays abstract while any one method is abstract -- so it did not catch
a single method losing @AbstractMethod, despite its docstring claiming
"missing any method". Assert the exact abstract set instead, which
fails if any of the five is no longer abstract.
Copilot AI lite review requested due to automatic review settings August 16, 2026 14:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

- Qualify the cross-module `project_camera` reference in
  `Perspective.to_screen`'s comment, now that it lives in a different
  module from `ViewState`.
- Note on `Perspective.eye_distance` that `view_distance` is the true
  eye only at full strength (the eye sits at `view_distance / strength`).
- Split the projection-variant contract test per variant, so a first
  failure no longer masks the rest.
Copilot AI review requested due to automatic review settings August 16, 2026 14:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@bjmorgan
bjmorgan merged commit 0455e38 into main Aug 16, 2026
5 checks passed
@bjmorgan
bjmorgan deleted the feature/projection-variants branch August 16, 2026 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants